feat: add GCP Parameter Manager and Secret Manager OpenFeature providers#1770
feat: add GCP Parameter Manager and Secret Manager OpenFeature providers#1770mahpatil wants to merge 5 commits intoopen-feature:mainfrom
Conversation
Adds two new OpenFeature Java providers backed by GCP-native storage: - `providers/gcp-parameter-manager`: Reads feature flags from Google Cloud Parameter Manager (google-cloud-parametermanager:0.31.0). Supports flag keys mapping directly to parameter names with optional prefix, configurable version (default "latest"), TTL-based in-memory cache (default 5 min, 500 entries), and Application Default Credentials with optional explicit GoogleCredentials. - `providers/gcp-secret-manager`: Reads feature flags from Google Cloud Secret Manager (google-cloud-secretmanager:2.57.0). Identical configuration surface with secretVersion, cacheExpiry, cacheMaxSize, and secretNamePrefix options. Both providers: - Implement all 5 OpenFeature evaluation methods (bool/string/int/double/object) - Parse JSON values into OpenFeature Value/MutableStructure for object flags - Map GCP NotFoundException → FlagNotFoundError, other errors → GeneralError - Pass all quality gates: Checkstyle, PMD, SpotBugs, Spotless - Include 41 unit tests each (FlagCache, FlagValueConverter, provider tests) - Include @tag("integration") tests gated by GCP_PROJECT_ID env var Registers both modules in root pom.xml, release-please-config.json, and .release-please-manifest.json at version 0.0.1. https://claude.ai/code/session_017KfN53GLayq9uci3NXuAn4
…oviders Creates two standalone Maven sample applications under samples/ to demonstrate end-to-end usage of the GCP OpenFeature providers against a real GCP project. samples/gcp-secret-manager-sample/ - SecretManagerSampleApp.java — evaluates 5 flag types (bool/string/int/double/object) using GcpSecretManagerProvider with ADC and a 30s TTL cache - setup.sh / teardown.sh — gcloud commands to create/delete the sample secrets - pom.xml — standalone Maven project with exec-maven-plugin - README.md — step-by-step instructions with expected output and troubleshooting samples/gcp-parameter-manager-sample/ - ParameterManagerSampleApp.java — identical scenario for GcpParameterManagerProvider with locationId="global" and parameterNamePrefix="of-sample-" - setup.sh / teardown.sh — gcloud commands for Parameter Manager API - pom.xml / README.md — same structure as Secret Manager sample Feature flags demonstrated (e-commerce scenario): dark-mode (bool), banner-text (string), max-cart-items (int), discount-rate (double), checkout-config (JSON object) Samples are NOT added to the root multi-module build (no quality gate overhead). Run `mvn install -DskipTests` from repo root first, then `mvn exec:java` in each sample. https://claude.ai/code/session_017KfN53GLayq9uci3NXuAn4
feat: add GCP Parameter Manager and Secret Manager OpenFeature providers
There was a problem hiding this comment.
Code Review
This pull request adds two new OpenFeature providers for Google Cloud: Parameter Manager and Secret Manager. Both implementations include TTL-based caching and support for standard flag types. The review feedback highlights several areas for improvement: ensuring the GCP clients are properly reset to null during shutdown to allow for re-initialization, adding support for configurable parameter versions in the Parameter Manager provider for consistency, and optimizing the O(N) cache eviction logic to prevent performance issues under high load.
| if (client != null) { | ||
| try { | ||
| client.close(); | ||
| } catch (Exception e) { | ||
| log.warn("Error closing ParameterManagerClient", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
The client field is not reset to null after being closed in the shutdown method. Since the initialize method (lines 80-82) only creates a new client if the field is null, calling initialize again after a shutdown will result in the provider attempting to use a closed client, leading to errors. Setting the field to null ensures that a fresh client is created upon re-initialization.
public void shutdown() {
if (client != null) {
try {
client.close();
} catch (Exception e) {
log.warn("Error closing ParameterManagerClient", e);
}
client = null;
}
log.info("GcpParameterManagerProvider shut down");
}| if (client != null) { | ||
| try { | ||
| client.close(); | ||
| } catch (Exception e) { | ||
| log.warn("Error closing SecretManagerServiceClient", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
The client field is not reset to null after being closed in the shutdown method. Since the initialize method (lines 80-82) only creates a new client if the field is null, calling initialize again after a shutdown will result in the provider attempting to use a closed client. Resetting the field to null allows for proper re-initialization.
public void shutdown() {
if (client != null) {
try {
client.close();
} catch (Exception e) {
log.warn("Error closing SecretManagerServiceClient", e);
}
client = null;
}
log.info("GcpSecretManagerProvider shut down");
}| public class GcpParameterManagerProviderOptions { | ||
|
|
||
| /** | ||
| * GCP project ID that owns the parameters. Required. | ||
| * Example: "my-gcp-project" or numeric project number "123456789". | ||
| */ | ||
| private final String projectId; | ||
|
|
||
| /** | ||
| * GCP location for the Parameter Manager endpoint. Optional. | ||
| * Use "global" (default) for the global endpoint, or a region such as "us-central1" | ||
| * when parameters are stored regionally. | ||
| */ | ||
| @Builder.Default | ||
| private final String locationId = "global"; | ||
|
|
||
| /** | ||
| * Explicit Google credentials to use when creating the Parameter Manager client. | ||
| * When {@code null} (default), Application Default Credentials (ADC) are used | ||
| * automatically by the GCP client library. | ||
| */ | ||
| private final GoogleCredentials credentials; | ||
|
|
||
| /** | ||
| * How long a fetched parameter value is retained in the in-memory cache before | ||
| * the next evaluation triggers a fresh GCP API call. | ||
| * | ||
| * <p>GCP Parameter Manager has API quotas. Set this to at least | ||
| * {@code Duration.ofSeconds(30)} in high-throughput scenarios. | ||
| * | ||
| * <p>Default: 5 minutes. | ||
| */ | ||
| @Builder.Default | ||
| private final Duration cacheExpiry = Duration.ofMinutes(5); | ||
|
|
||
| /** | ||
| * Maximum number of distinct parameter names held in the cache at once. | ||
| * When the cache is full, the oldest entry is evicted before inserting a new one. | ||
| * Default: 500. | ||
| */ | ||
| @Builder.Default | ||
| private final int cacheMaxSize = 500; | ||
|
|
||
| /** | ||
| * Optional prefix prepended to every flag key before constructing the GCP | ||
| * parameter name. For example, setting {@code parameterNamePrefix = "ff-"} maps | ||
| * flag key {@code "my-flag"} to parameter name {@code "ff-my-flag"}. | ||
| */ | ||
| private final String parameterNamePrefix; |
There was a problem hiding this comment.
The PR description mentions that the gcp-parameter-manager provider supports a configurable version (defaulting to "latest"), similar to the gcp-secret-manager provider. However, the GcpParameterManagerProviderOptions class is missing a field for this configuration, and the provider implementation (line 165 in GcpParameterManagerProvider.java) has the version hardcoded to "latest". Consider adding a parameterVersion field to this options class to align with the intended functionality and the Secret Manager implementation.
| private void evictOldest() { | ||
| store.entrySet().stream() | ||
| .min(Comparator.comparing(e -> e.getValue().expiresAt)) | ||
| .map(Map.Entry::getKey) | ||
| .ifPresent(store::remove); | ||
| } |
There was a problem hiding this comment.
The evictOldest method performs a full scan of the ConcurrentHashMap (maxSize of 500, this will become a performance bottleneck if the cache size is increased or if evictions occur frequently under high load. Consider using a data structure that maintains order (like a LinkedHashMap with access order, though it requires external synchronization) or a dedicated caching library like Caffeine if more advanced eviction is needed.
|
hello, and thank you for your contributions, this looks like a solid implementations, but can you please split this up into two separate pull requests, so we can evaluate each provider on its own. For us maintainers it is easier, when a pull request is small, and provides just one additional feature. Hence i highly recommend to split this up into two dedicated pull requests, one per provider. Thank you. |
|
Thanks for the review feedback! I've addressed all the code assist comments:
|
thanks will do so next |
This PR
Adds two new OpenFeature Java providers backed by GCP-native storage:
Related Issues
Fixes #1420
Notes
Follow-up Tasks
How to test
Included a samples folder with test code to create the necessary config in GCP and tests to be executed.